feat: add the validate utils #32
Conversation
|
hi @duguorong009 please take a look at the CI errors when you get a minute |
|
sorry for the delay getting back to you, its been a hectic week |
src/lib.rs
Outdated
| @@ -29,6 +29,64 @@ fn jsonschema_is_valid(schema: Json) -> bool { | |||
| } | |||
| } | |||
|
|
|||
| #[pg_extern(immutable, strict)] | |||
| fn validate_json_schema(schema: Json, instance: Json) -> bool { | |||
There was a problem hiding this comment.
From the linked issue I thought the goal was to produce functions that return an array of strings that described and errors that are present.
These two functions have identical signatures and behavior to json_matches_schema and jsonb_matches_schema but have raise notices that list the errors.
If that is the goal, why not update the existing two functions? e.g. is it a performance problem? if so, how big is the penalty?
If it is something like performance, an optional argument to the existing functions like on_error_raise that takes one of nothing (default), notice, or exception would be preferred to adding new functions
There was a problem hiding this comment.
Yes, there is performance problem.
From the doc, the jsonschema::is_valid is much faster than validate function.
Anyhow, I removed the new functions, and updated the existing functions as recommended.
Please re-check it. @olirice
|
@duguorong009 sorry for the back and forth It's not clear to me what we're solving here. #14 requested exposing Given that there is a performance hit for using But check constraints won't bubble up those hints, logs would need to be set to If you'd be interested in changing this to a single immutable function (json only, not jsonb) fn jsonschema_validation_errors(schema: Json, instance: Json) -> Vec<String>that returns an array of errors or an empty array (if there are no errors) I'd be happy to approve thanks! |
Thanks for the detailed tips! @olirice |
|
looks good please include tests for 0 errors, 1 error, and multiple errors: e.g. // No errors
crate::jsonschema_validation_errors(
Json(json!({ "maxLength": 4 })),
Json(json!("foo")),
)
// One error
crate::jsonschema_validation_errors(
Json(json!({ "maxLength": 4 })),
Json(json!("123456789")),
)
// Multiple errors
crate::jsonschema_validation_errors(
Json(json!(
{
"type": "object",
"properties": {
"foo": {
"type": "string"
},
"bar": {
"type": "number"
},
"baz": {
"type": "boolean"
},
"additionalProperties": false,
"required": ["foo", "bar", "baz"]
}
})),
Json(json!({"foo": 1, "bar": [], "bat": true})),
) |
src/lib.rs
Outdated
| })), | ||
| Json(json!({"foo": 1, "bar": [], "bat": true})), | ||
| ); | ||
| assert!(errors.len() == 1); |
There was a problem hiding this comment.
there should be multiple errors on this one
There was a problem hiding this comment.
Thanks for comment!
Will update! @olirice
|
thanks! |
What kind of change does this PR introduce?
Add the
validateutils for getting the actual validation errors thrown while validating the json schema and instance.What is the current behavior?
Closes #14
What is the new behavior?
Receives the actual errors thrown, as
Vec<String>Additional context